Upload data api three layer refactor#14798
Conversation
🦋 Changeset detectedLatest commit: 2e18fed The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
9726d21 to
97d6321
Compare
* fix: update change set config. * add change set back
| return Buffer.from(input, 'utf-8').toString('base64'); | ||
| } | ||
|
|
||
| return Buffer.from(input.buffer).toString('base64'); |
There was a problem hiding this comment.
Both the React Native and server toBase64 implementations ignore byteOffset and byteLength. The browser implementation correctly uses new Uint8Array(input.buffer, input.byteOffset, input.byteLength)
There was a problem hiding this comment.
Updated both implementation (server & RN ) ✅
bobbor
left a comment
There was a problem hiding this comment.
looking at the 3 files (client/utils/toBase64.ts, client/utils/toBase64.native.ts and server/utils/toBase64.ts) they all do the same and can result in a foundation/utils/toBase64.ts.
the client file uses low-level constructs (Uint8Array, TextEncoder and btoa) which are available in all environments.
so I'd suggest:
- take the
client/util/toBase64.tsmove that tofoundation/util/toBase64.ts+ test - remove the native and server implementations + tests
- remove the
toBase64from theFoundationContext
Replace the per-environment client/server toBase64 implementations with a single foundation/utils/toBase64.ts. The browser-style impl using TextEncoder/btoa/Uint8Array works in all supported runtimes (Node 18+, modern browsers, React Native), so toBase64 no longer needs DI through FoundationContext.
Done ✅ |
Description of changes
Refactor the internal
uploadDataAPI in@aws-amplify/storageto follow thethree-layer architecture (foundation / client / server):
foundation/— environment-agnostic core. ContainscalculateContentCRC32and
calculateContentMd5, which now receive aFoundationContext(
{ amplify, readFile, toBase64 }) via dependency injection instead ofbranching on the runtime.
client/— browser + React Native implementations ofreadFileandtoBase64(with.native.tssplits preserved), plus the publicclient-side
uploadDataAPI.server/— Node.js implementations and the public server-sideuploadDataAPI, which injects the request-scoped Amplify instance fromgetAmplifyServerContext.The
uploadDatacall graph (putObjectJob,getMultipartUploadHandlers,uploadPartExecutor,loadOrCreateMultipartUpload) now threadsFoundationContextinstead ofAmplifyClassV6, so the foundation layer isfully free of environment-discriminating logic.
Public API surface is unchanged. Routing to the correct bundle per
environment is handled via conditional exports in
package.json(
browser/import/require), and the existingaws-amplify/storage/serversubpath is preserved for backward compatibility.Legacy
providers/s3/utils/{crc32,md5,readFile}.tsare kept as shims fordeleteObjectsuntilremoveis migrated; new code imports fromfoundation/utilsandclient|server/utilsinstead.This is the first API migrated to the three-layer pattern; follow-up PRs
will apply the same split to the remaining Storage APIs.
Issue #, if available
N/A — internal refactor, no customer-facing behavior change.
Description of how you validated changes
uploadDataupdated to passFoundationContextinstead of
Amplify; all continue to pass.foundation/utils/*,client/utils/*, andserver/utils/*modules. New code has 100%statement/function/line coverage.
aws-amplifyJest config now runs two projects so the umbrellapackage's exports are verified under both resolution paths:
nodeproject (pinnedcustomExportConditions: ['node']) — existingtests, asserts the SSR surface of
@aws-amplify/storageinexports.test.ts.browserproject (jsdom defaults) — newexports.browser.test.tsasserts the full client-side surface (
uploadData,downloadData,remove,list,getProperties,copy,getUrl,isCancelError,StorageError,DEFAULT_PART_SIZE).babel-jestwith@babel/preset-env(modules: 'commonjs') is usedto transform the
.mjsESM bundle emitted by@aws-amplify/storageso Jest can load it under the browser condition; ts-jest continues to
handle
.ts/.js.predictionsJest config no longer pins thenodeexport condition.It provides a global
moduleNameMapperstub for@aws-amplify/storage(predictions only consumes
getUrl), which removes the need for aresolution pin. Existing per-test
jest.mock(...)overrides still work.yarn testpasses for thestorage,aws-amplify, andpredictionspackages. Package-level coverage thresholds respected.
Checklist
yarn testpassesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.